Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/source site polymorphism #115

Merged

Conversation

anaximander23
Copy link
Contributor

Addresses #58
Lays some groundwork for #47
Should streamline the process of adding new supported sources for minis.

anaximander23 and others added 30 commits December 10, 2019 11:10
…e multiple sources, so that we can represent mirrors and duplicates.
@anaximander23 anaximander23 force-pushed the feature/source-site-polymorphism branch from ee66307 to 6e6a673 Compare January 13, 2020 11:14
@anaximander23 anaximander23 force-pushed the feature/source-site-polymorphism branch from b4e7518 to 288328f Compare January 13, 2020 14:25
@aluhrs13
Copy link
Owner

aluhrs13 commented Jan 13, 2020

  • Every Mini added adds a record to SourceSite, regardless of existing SourceSite (test with onmioji's Thingiverse and mz4250's Shapways)
  • Trying to add a Mini from a creator's page (/Minis/Create?URL=) doesn't work.
  • /Minis/Details still uses Mini.Link instead of MiniSourceSite. I'm thinking that the styling should be done by CSS class. Add a class using SiteName, then when adding a source, add CSS to do styling for the button. Much cleaner than a switch.
  • /Minis/Index, /Creators/Details still use Mini.Link instead of MiniSourceSite
  • #-style links from Gumroad not parsing (https://gumroad.com/nafarrate#jJbmb)
  • Double-check migration worked right for Gumroad, creators that previously had a gumroad as their "website" are showing those as website still and could cause issues. Probably manually fix?
  • New Thingiverse Minis are added with an API link.

Works:

  • Adding a Mini with an existing creator - Thingiverse, Shapeways, Gumroad (issues Duplicate mini logic #1 and User authentication #5 above)
  • Adding an existing Mini - Thingiverse, Shapeways, Gumroad
  • Adding a Mini with a new creator - Thingiverse, Gumroad
  • Viewing a Mini and opening external link (not using new code though, Shapeways Support #3 above)
  • Approving a Mini
  • Viewing a Creator (minor issue, #2 above)
  • Tagging

Polish follow-ups (probably do after merging to not make this PR any bigger):

  • Add a dropdown when more than one MiniSourceSite
  • Add tests for all parsers
  • Making parsing logic more rigid to handle if there's another thing in the query string like a social media link (this was weak before too)

@aluhrs13
Copy link
Owner

Opened #119 for figuring out the future of Mini.Link, #120 to go through and clean-up Gumroad and MyMiniFactory websites once we merge and deploy all this.

Just pushed a fix for some MyMiniFactory issues I was seeing. I still saw some duplicated SourceSites for MyMiniFactory, but I'm not sure if that was due to me messing up the DB, the SourceSite itself, or the submission handler. I'm going to reset the PPE DB to a recent backup to check.

…h CreatorUserName and SitName. However currentSource.Site.SiteName is always blank for some reason...
@aluhrs13 aluhrs13 mentioned this pull request Jan 15, 2020
6 tasks
@aluhrs13
Copy link
Owner

aluhrs13 commented Jan 15, 2020

Did a diff of the production and preproduction databases to figure out more about #119, it looks like MiniSourceSite.Link is NULL for all Gumroad Minis that were in the db pre-migration.

Query:

/* PPE */
SELECT
	Mini.ID,
	Mini.Name,
	MSS.Link,
	Creator.Name
  FROM [dbo].[Mini] AS Mini
  LEFT OUTER JOIN [dbo].[Creator] AS Creator
	ON Creator.ID = Mini.CreatorID
  LEFT OUTER JOIN [dbo].[MiniSourceSite] AS MSS
	ON MSS.MiniID=Mini.ID
	ORDER BY Mini.ID

MSS.Link should never be NULL, but is NULL for all Gumroad links

@aluhrs13
Copy link
Owner

aluhrs13 commented Jan 16, 2020

Only remaining open issues I have in my head right now:

  • MiniSourceSite.Link for Gumroad are NULL
  • Need to re-enable Facebook (mostly checklist for finalizing the PR)
  • Trying to add a Mini from a creator's page (/Minis/Create?URL=) doesn't work.

I'll try to do another thorough test pass in the next day or so.

@anaximander23
Copy link
Contributor Author

The reason that adding a mini from a creator's page doesn't work is because it's making a GET request; technically speaking a GET shouldn't have side effects like creating things in the database. One simple way to make this work would be to make the button a form submit so that it can POST instead.

@aluhrs13 aluhrs13 merged commit 417ba7d into aluhrs13:master Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants